Skip to content

Conversation

@major
Copy link
Contributor

@major major commented Jan 22, 2026

Description

Add comprehensive end-to-end tests for the rh-identity authentication module. These tests cover critical validation paths in src/authentication/rh_identity.py, ensuring proper error handling for malformed headers, missing fields, and entitlement validation.

Test Scenarios (9 total)

Header-level validation:

  • Missing x-rh-identity header (401)

Identity structure validation:

  • Missing identity field (400)

User identity validation:

  • Missing user_id (400)
  • Missing username (400)

System identity validation:

  • Missing cn (400)

Entitlement validation:

  • Missing required entitlement (403)
  • Entitlement with is_entitled=false (403)

Success cases:

  • Valid User identity with required entitlements (200) - validates response body
  • Valid System identity with required entitlements (200) - validates response body

Type of change

  • End to end tests improvement

Tools used to create PR

  • Assisted-by: Claude (Anthropic)
  • Generated by: N/A

Related Tickets & Documents

  • Related Issue # N/A
  • Closes # N/A

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  1. Run the e2e tests with rh-identity feature:

    uv run behave tests/e2e/features/authorized_rh_identity.feature
  2. Or run all e2e tests:

    uv run make test-e2e
  3. All 9 scenarios should pass, validating each error path and success case in the rh-identity authentication module.

Summary by CodeRabbit

  • Tests
    • Added comprehensive end-to-end coverage for RH Identity authentication: header formats, identity field validation, entitlement checks, and positive/negative flows; new feature included in test run.
    • Added helpers to construct and base64-encode RH Identity headers for varied test cases.
  • Chores
    • Added mode-specific test configurations and test-harness setup/teardown to enable RH Identity scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 22, 2026

Walkthrough

Adds end-to-end RH Identity auth support: two mode-specific Lightspeed config files, a Gherkin feature exercising x-rh-identity scenarios, environment hooks to swap configs and restart the service during RHIdentity-tagged tests, and step helpers to construct/encode x-rh-identity headers for tests.

Changes

Cohort / File(s) Summary
Configuration Files
tests/e2e/configuration/library-mode/lightspeed-stack-auth-rh-identity.yaml, tests/e2e/configuration/server-mode/lightspeed-stack-auth-rh-identity.yaml
New LCS YAML configs enabling RH Identity auth: server settings, logging, worker counts, library vs server modes, user-data collection paths, SQLite conversation cache, and required entitlements.
Feature Test Definition
tests/e2e/features/authorized_rh_identity.feature
New Gherkin feature covering missing/invalid/malformed x-rh-identity headers, User/System identity success cases, entitlement checks, and field validation errors.
Test Environment Setup
tests/e2e/features/environment.py
Adds RHIdentity feature-tag hooks that backup current config, apply mode-specific RH Identity config, restart lightspeed-stack, and restore config after the feature.
Test Step Implementations
tests/e2e/features/steps/auth.py
Adds _encode_rh_identity helper and steps to set/remove x-rh-identity headers in raw/base64/JSON/User/System forms; adds Authorization header removal helper and minor doc/log refinement.
Test Registry
tests/e2e/test_list.txt
Registers the new feature file in the E2E test list.

Sequence Diagram(s)

(omitted — changes are test/config additions and do not introduce a new multi-component runtime control flow requiring visualization)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Possibly related PRs

Suggested labels

ok-to-test

Suggested reviewers

  • tisnik
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'test(e2e): add comprehensive e2e tests for rh-identity authentication' clearly and specifically summarizes the main change—adding end-to-end tests for RH Identity authentication.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tisnik tisnik requested a review from radofuchs January 23, 2026 14:16
Add configuration files for e2e testing with rh-identity authentication
module enabled for both server-mode and library-mode deployments.

Both configs require the 'rhel' entitlement for validation testing.

Signed-off-by: Major Hayden <major@redhat.com>
@major major force-pushed the e2e-rh-identity-auth-tests branch from 686d53b to 266e6d4 Compare January 26, 2026 16:29
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@tests/e2e/features/authorized_rh_identity.feature`:
- Around line 1-163: Add two E2E scenarios to the existing
authorized_rh_identity.feature to cover invalid base64 and invalid JSON
handling: one scenario using the step set_rh_identity_header_raw() to set a
non-base64 string in x-rh-identity and another using
set_rh_identity_header_base64_raw() to set a base64 string that decodes to
invalid JSON; both should POST to the "authorized" endpoint and assert a 400
status code and that the response body contains the appropriate error message
(e.g., "Invalid base64 in x-rh-identity header" for the raw case and "Invalid
JSON in x-rh-identity header" for the decoded JSON case) so the BDD feature
matches the unit-tested behavior.

In `@tests/e2e/features/steps/auth.py`:
- Around line 132-138: The entitlements parsing currently adds an empty-string
key when the entitlements cell is blank; update the code that builds
entitlements (the loop over fields["entitlements"].split(",") which trims into
ent) to skip any ent that is empty after strip (e.g., continue if not ent), so
only non-empty entitlement names are added to the entitlements dict (apply the
same guard to the other identical block that populates entitlements).

@major major force-pushed the e2e-rh-identity-auth-tests branch from 266e6d4 to 32aed2f Compare January 26, 2026 16:39
major added 2 commits January 26, 2026 10:39
Add step definitions to set x-rh-identity headers in various formats:
- Raw string values (for invalid base64 testing)
- Base64-encoded raw strings (for invalid JSON testing)
- Base64-encoded JSON objects
- Valid User identity with configurable fields
- Valid System identity with configurable fields

Includes helper function to encode identity data to base64.

Signed-off-by: Major Hayden <major@redhat.com>
Register @RHIdentity tag in before_feature and after_feature hooks to
switch configuration to rh-identity auth mode during feature execution
and restore the original configuration afterwards.

Signed-off-by: Major Hayden <major@redhat.com>
@major
Copy link
Contributor Author

major commented Jan 26, 2026

@radofuchs I think this matches what we met about earlier. :)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/e2e/features/steps/auth.py (1)

25-35: Avoid logging full Authorization header values.

Line 35 prints the entire header; this can leak secrets in CI logs. Prefer redaction.

🔒 Proposed redaction
-    print(f"🔑 Set Authorization header to: {header_value}")
+    token_type = header_value.split(" ", 1)[0] if header_value else "<empty>"
+    print(f"🔑 Set Authorization header to: {token_type} <redacted>")
♻️ Duplicate comments (1)
tests/e2e/features/authorized_rh_identity.feature (1)

1-163: Add invalid base64/JSON scenarios for x-rh-identity.

The feature still lacks explicit cases for malformed base64 and invalid JSON payloads.

@major major force-pushed the e2e-rh-identity-auth-tests branch from 32aed2f to 8ca8553 Compare January 26, 2026 17:30
Add comprehensive e2e test scenarios covering all validation paths in
the rh-identity authentication module:

- Missing x-rh-identity header (401)
- Invalid base64 encoding (400)
- Invalid JSON content (400)
- Missing/null identity field (400)
- Missing identity type field (400)
- Unsupported identity type (400)
- User identity: missing user field (400)
- User identity: missing user_id (400)
- User identity: missing username (400)
- System identity: missing system field (400)
- System identity: missing cn (400)
- System identity: missing account_number (400)
- Missing required entitlements (403)
- Empty entitlements (403)
- Entitlement with is_entitled=false (403)
- Valid User identity with entitlements (200)
- Valid System identity with entitlements (200)

Signed-off-by: Major Hayden <major@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant